resolve main class automatically#71
Merged
Merged
Conversation
Signed-off-by: xuzho <xuzho@microsoft.com>
andxu
reviewed
Oct 11, 2017
| List<String> projectNames; | ||
| if (arguments.size() > 1 && arguments.get(1) != null) { | ||
| // project name specified | ||
| projectNames = new ArrayList<String>(); |
andxu
reviewed
Oct 11, 2017
| projects.add(rootElm.elementText("name")); | ||
| } catch (DocumentException e) { | ||
| // TODO Auto-generated catch block | ||
| e.printStackTrace(); |
Contributor
Author
There was a problem hiding this comment.
i don't see any close/dispose method for Reader/document
andxu
reviewed
Oct 11, 2017
| e.printStackTrace(); | ||
| } | ||
| } | ||
| projects.add(null); |
Contributor
Author
There was a problem hiding this comment.
fall back to workspace scope
andxu
reviewed
Oct 11, 2017
| IJavaSearchScope searchScope = createSearchScope(projectName); | ||
| SearchPattern pattern = SearchPattern.createPattern("main(String[]) void", IJavaSearchConstants.METHOD, | ||
| IJavaSearchConstants.DECLARATIONS, SearchPattern.R_CASE_SENSITIVE | SearchPattern.R_EXACT_MATCH); | ||
| ArrayList<String> uris = new ArrayList<String>(); |
Contributor
There was a problem hiding this comment.
List<String> uris = new ArrayList<>()
Signed-off-by: xuzho <xuzho@microsoft.com>
| private List<String> resolveProjectName(List<String> projectFiles) { | ||
| List<String> projects = new ArrayList<>(); | ||
| for (String f : projectFiles) { | ||
| SAXReader reader = new SAXReader(); |
Contributor
There was a problem hiding this comment.
Does eclipse have any built-in utility to help parse .project file?
| SearchEngine searchEngine = new SearchEngine(); | ||
| searchEngine.search(pattern, new SearchParticipant[] {SearchEngine.getDefaultSearchParticipant()}, | ||
| searchScope, requestor, null /* progress monitor */); | ||
| return uris.size() == 0 ? null : uris.get(0); |
Contributor
There was a problem hiding this comment.
if there are multiple main class in the target project, should pop up a dropdown menu in UI to tell user to choose one.[#Pending]
Contributor
There was a problem hiding this comment.
I think we should return multiple to let js side has the possibilities to choose.
andxu
previously approved these changes
Oct 12, 2017
Signed-off-by: xuzho <xuzho@microsoft.com>
andxu
reviewed
Oct 17, 2017
| } | ||
| if (o instanceof ResolutionItem) { | ||
| ResolutionItem item = (ResolutionItem) o; | ||
| return item.mainClass == this.mainClass && item.projectName == this.projectName; |
Contributor
There was a problem hiding this comment.
in java == is not for equal test.
Contributor
Author
There was a problem hiding this comment.
nice catch, i should use equals instead
andxu
reviewed
Oct 17, 2017
| if (project != null) { | ||
| String mainClass = method.getDeclaringType().getFullyQualifiedName(); | ||
| String projectName = ProjectsManager.DEFAULT_PROJECT_NAME.equals(project.getName()) ? null : project.getName(); | ||
| uris.add(new ResolutionItem(mainClass, projectName)); |
andxu
reviewed
Oct 17, 2017
| * @return main class and project name | ||
| * resolve main class and project name. | ||
| * @return an array of main class and project name | ||
| * @throws CoreException when there are error when resolving main class. |
andxu
reviewed
Oct 17, 2017
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return mainClass.hashCode() * 13 + (projectName == null ? 0 : projectName.hashCode()); |
Contributor
There was a problem hiding this comment.
missing the handle case for null mainClass and null projectName
| IJavaSearchScope searchScope = SearchEngine.createWorkspaceScope(); | ||
| SearchPattern pattern = SearchPattern.createPattern("main(String[]) void", IJavaSearchConstants.METHOD, | ||
| IJavaSearchConstants.DECLARATIONS, SearchPattern.R_CASE_SENSITIVE | SearchPattern.R_EXACT_MATCH); | ||
| ArrayList<ResolutionItem> uris = new ArrayList<>(); |
Signed-off-by: xuzho <xuzho@microsoft.com>
andxu
previously approved these changes
Oct 18, 2017
Signed-off-by: xuzho <xuzho@microsoft.com>
| return resolveMainClassCore(); | ||
| } | ||
|
|
||
| private List<ResolutionItem> resolveMainClassCore() throws CoreException { |
Contributor
There was a problem hiding this comment.
new method resolveMainClassCore is unnecessary.[pending]
testforstephen
approved these changes
Oct 18, 2017
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Signed-off-by: xuzho xuzho@microsoft.com
microsoft/vscode-java-debug#9